-
Notifications
You must be signed in to change notification settings - Fork 4.2k
feat(api,worker): skip sending messages outside of the subscribers schedule fixes NV-6618 #9126
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(api,worker): skip sending messages outside of the subscribers schedule fixes NV-6618 #9126
Conversation
…-global-preference-schedule
…16-inbox-schedule
…-and-default-schedule
…v-6617-dashboard-subscribers-schedule
| @@ -1,4 +1,5 @@ | |||
| import { | |||
| GetSubscriberSchedule, | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved to the application-generic
| IS_STEP_RUN_LOGS_WRITE_ENABLED=true | ||
| IS_WORKFLOW_RUN_LOGS_WRITE_ENABLED=true | ||
| IS_NOTIFICATION_SEVERITY_ENABLED=true | ||
| IS_SUBSCRIBERS_SCHEDULE_ENABLED=true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for the worker logic to work when running e2e tests
| environment: { _id: job._environmentId }, | ||
| }); | ||
|
|
||
| if (isSubscribersScheduleEnabled && !this.shouldSkipScheduleCheck(job, notification)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the trigger, in-app, digest, delay and critical workflows will be "skipped" when checking the schedule
| { readPreference: 'secondaryPreferred' } | ||
| ); | ||
|
|
||
| if (!isWithinSchedule(schedule, new Date(), subscriber?.timezone)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If not within the schedule (outside), then cancel the job and provide the step_run and trace details
| import { expect } from 'chai'; | ||
| import { isWithinSchedule } from './schedule-validator'; | ||
|
|
||
| describe('ScheduleValidator', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unit tests to cover isWithinSchedule logic
| const daysToCheck = [currentDay]; | ||
|
|
||
| // For overnight schedules, also check the previous day | ||
| const previousDay = getPreviousDay(currentDay); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the overnight schedules are for example happening in this case when:
- hours are set
monday 11:00 PM - 2:00 AM - current time is
tuesday 1:00 AM
so we have to take a look at the schedule of the previous day
…ip-messages-outside-of-the-subscribers-schedule
✅ Deploy preview added
To edit notification comments on pull requests, go to your Netlify project configuration. |
| const schedule = await this.getSubscriberSchedule.execute( | ||
| GetSubscriberScheduleCommand.create({ | ||
| environmentId: job._environmentId, | ||
| organizationId: job._organizationId, | ||
| _subscriberId: job._subscriberId, | ||
| }) | ||
| ); | ||
|
|
||
| const subscriber = await this.subscriberRepository.findOne( | ||
| { | ||
| _id: job._subscriberId, | ||
| _environmentId: job._environmentId, | ||
| _organizationId: job._organizationId, | ||
| }, | ||
| 'timezone', | ||
| { readPreference: 'secondaryPreferred' } | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a general note, if the subscriber is refetched again in the schedule usecase, or soemwhere up the scope, better to reuse it to avoid a duplicate db call if possible. (not sure if it's the case here)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those two were made to be lightweight:
- the
getSubscriberScheduleonly uses one request to read preferences, nothing more - the
this.subscriberRepository.findOneis done tosecondaryreplica and only fetchtimezone
What changed? Why was the change needed?
Worker: skip sending messages when outside of the subscriber's schedule.
Screenshots
Screen.Recording.2025-09-11.at.16.06.30.mp4
Screen.Recording.2025-09-11.at.16.17.27.mov